Skip to content

[jsweep] Clean messages_core.cjs#20630

Merged
pelikhan merged 1 commit intomainfrom
jsweep/clean-messages-core-66fc3221ee59a527
Mar 12, 2026
Merged

[jsweep] Clean messages_core.cjs#20630
pelikhan merged 1 commit intomainfrom
jsweep/clean-messages-core-66fc3221ee59a527

Conversation

@github-actions
Copy link
Contributor

Summary

Cleaned actions/setup/js/messages_core.cjs and added a comprehensive test suite.

Context type: github-script (uses core global)


Changes

messages_core.cjs

  1. Moved lazy require() to top-level importerror_helpers.cjs was required inside getMessages() on every call. There's no circular dependency, so it belongs at the module top level like all other imports.

  2. Simplified toSnakeCase() with Object.fromEntries + flatMap — replaced the imperative for loop + two assignments with a functional one-liner. The new version also avoids duplicate entries when a key is already in snake_case (e.g. run_url previously got assigned twice):

    // Before
    const result = {};
    for (const [key, value] of Object.entries(obj)) {
      const snakeKey = key.replace(/([A-Z])/g, "_$1").toLowerCase();
      result[snakeKey] = value;
      result[key] = value;  // duplicate if already snake_case
    }
    
    // After
    return Object.fromEntries(
      Object.entries(obj).flatMap(([key, value]) => {
        const snakeKey = key.replace(/([A-Z])/g, "_$1").toLowerCase();
        return snakeKey === key ? [[key, value]] : [[snakeKey, value], [key, value]];
      })
    );

messages_core.test.cjs (new file)

Added 17 tests covering all three exported functions:

Group Tests
renderTemplate single placeholder, multiple placeholders, missing key preserved, undefined value preserved, numeric coercion, boolean coercion, no placeholders, empty template
toSnakeCase camelCase→snake_case, original key preserved, no duplicates for snake_case input, multi-word conversion, empty object, mixed input
getMessages null when unset, null when empty, valid JSON parsed, multiple fields parsed, invalid JSON warns + returns null

Validation ✅

  • Formatting: npm run format:cjs
  • Linting: npm run lint:cjs
  • Type checking: npm run typecheck
  • Tests: npm run test:js ✓ (all 17 new tests pass; existing failures are pre-existing environment issues unrelated to this change)

Generated by jsweep - JavaScript Unbloater ·

Warning

⚠️ Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • proxy.golang.org

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "proxy.golang.org"

See Network Configuration for more information.

  • expires on Mar 14, 2026, 3:31 AM UTC

- Move lazy require() of error_helpers.cjs to top-level import
- Simplify toSnakeCase() using Object.fromEntries + flatMap (avoids duplicate entries for already-snake_case keys)
- Add comprehensive test file messages_core.test.cjs with 17 tests covering renderTemplate, toSnakeCase, and getMessages

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions
Copy link
Contributor Author

🤖 Contribution Check — [jsweep] Clean messages_core.cjs

Nice work on this cleanup! The messages_core.cjs refactor is tight and well-reasoned — promoting the lazy require() to the module top level is the right call, and the flatMap/Object.fromEntries rewrite of toSnakeCase is cleaner while also fixing the latent duplicate-key bug for already-snake_case inputs. The 17-test suite covers all three exported functions with good boundary cases.

Checklist summary:

Check Result
On-topic ✅ yes — aligns with agentic JS cleanup tooling
Follows process ✅ yes — agentic PR via core team automation
Focused ✅ yes — single module refactor + tests
New dependencies ✅ no
Has tests ✅ yes — 17 tests across all exported functions
Has description ✅ yes — thorough summary with before/after code
Lines changed 198 (184 additions, 14 deletions)

Verdict: 🟢 Aligned — this PR looks ready for maintainer review!

Generated by Contribution Check ·

@pelikhan pelikhan marked this pull request as ready for review March 12, 2026 13:48
Copilot AI review requested due to automatic review settings March 12, 2026 13:48
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Cleans up messages_core.cjs by hoisting a lazy require() to the module top level and simplifying toSnakeCase() using Object.fromEntries/flatMap, while also fixing a subtle duplicate-entry bug for already-snake_case keys. A comprehensive test suite is added.

Changes:

  • Moved require("./error_helpers.cjs") from inside getMessages() to the top of the module.
  • Rewrote toSnakeCase() as a functional one-liner that avoids duplicate entries for keys already in snake_case.
  • Added 17 new tests covering renderTemplate, toSnakeCase, and getMessages.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
actions/setup/js/messages_core.cjs Hoisted import; simplified toSnakeCase(); updated JSDoc comments.
actions/setup/js/messages_core.test.cjs New test suite covering all three exported functions.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

@pelikhan pelikhan merged commit f151d2d into main Mar 12, 2026
4 checks passed
@pelikhan pelikhan deleted the jsweep/clean-messages-core-66fc3221ee59a527 branch March 12, 2026 14:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants